[PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming

Dikshita Agarwal posted 24 patches 4 months ago
[PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming
Posted by Dikshita Agarwal 4 months ago
Previously, the transition to load resources substate was only allowed
when instance state was INIT, since capture port streaming was not
permitted before output port streaming in the driver.
However, now after removing that restriction, the instance state can be
OUTPUT_STREAMING when firmware start is triggered and substate needs to
be moved to LOAD_RESOURCES.

Fixes: 547f7b8c5090 ("media: iris: add check to allow sub states transitions")
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Tested-by: Vikash Garodia <quic_vgarodia@quicinc.com> # X1E80100
Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
---
 drivers/media/platform/qcom/iris/iris_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_state.c b/drivers/media/platform/qcom/iris/iris_state.c
index 104e1687ad39dab93ff66450ba3a97c309b1e1e1..a21238d2818f9606871953bd0bee25382cca0474 100644
--- a/drivers/media/platform/qcom/iris/iris_state.c
+++ b/drivers/media/platform/qcom/iris/iris_state.c
@@ -122,7 +122,8 @@ static bool iris_inst_allow_sub_state(struct iris_inst *inst, enum iris_inst_sub
 		return false;
 	case IRIS_INST_OUTPUT_STREAMING:
 		if (sub_state & (IRIS_INST_SUB_DRC_LAST |
-			IRIS_INST_SUB_DRAIN_LAST | IRIS_INST_SUB_OUTPUT_PAUSE))
+			IRIS_INST_SUB_DRAIN_LAST | IRIS_INST_SUB_OUTPUT_PAUSE |
+			IRIS_INST_SUB_LOAD_RESOURCES))
 			return true;
 		return false;
 	case IRIS_INST_STREAMING:

-- 
2.34.1
Re: [PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming
Posted by Bryan O'Donoghue 4 months ago
On 13/08/2025 10:37, Dikshita Agarwal wrote:
> However, now after removing that restriction, the instance state can be
> OUTPUT_STREAMING when firmware start is triggered and substate needs to
> be moved to LOAD_RESOURCES.
> 
> Fixes: 547f7b8c5090 ("media: iris: add check to allow sub states transitions")

If a restriction has been removed, has that restriction been marked as a 
Fixes: ?

If not then we can't actually backport this fix as the dependency - the 
restriction removal has not been marked for backport.

Please evaluate if the necessary change you have stipulated here has 
been marked for backport with Fixes: and if so mention the appropriate 
commit SHA in your commit log.

If you are referring to the commit immediately preceding this patch "Fix 
buffer count reporting in internal buffer check" then again you should 
mention that in the log so _which_ dependency you mean is clear.

If I were trying to follow this series on some kind of -stable kernel, I 
don't think this commit log would reasonably tell me which depends I 
need as an antecedent.

---
bod
Re: [PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming
Posted by Konrad Dybcio 4 months ago
On 8/13/25 11:51 PM, Bryan O'Donoghue wrote:
> On 13/08/2025 10:37, Dikshita Agarwal wrote:
>> However, now after removing that restriction, the instance state can be
>> OUTPUT_STREAMING when firmware start is triggered and substate needs to
>> be moved to LOAD_RESOURCES.
>>
>> Fixes: 547f7b8c5090 ("media: iris: add check to allow sub states transitions")
> 
> If a restriction has been removed, has that restriction been marked as a 
> Fixes: ?
> 
> If not then we can't actually backport this fix as the dependency - the 
> restriction removal has not been marked for backport.

Please stop confusing fixes and backports..

If you're really paranoid about this commit getting autoselected, see
'noautosel' under this section:

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Konrad

> 
> Please evaluate if the necessary change you have stipulated here has 
> been marked for backport with Fixes: and if so mention the appropriate 
> commit SHA in your commit log.
> 
> If you are referring to the commit immediately preceding this patch "Fix 
> buffer count reporting in internal buffer check" then again you should 
> mention that in the log so _which_ dependency you mean is clear.
> 
> If I were trying to follow this series on some kind of -stable kernel, I 
> don't think this commit log would reasonably tell me which depends I 
> need as an antecedent.
> 
> ---
> bod
> 
>
Re: [PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming
Posted by Krzysztof Kozlowski 4 months ago
On 14/08/2025 00:55, Konrad Dybcio wrote:
> On 8/13/25 11:51 PM, Bryan O'Donoghue wrote:
>> On 13/08/2025 10:37, Dikshita Agarwal wrote:
>>> However, now after removing that restriction, the instance state can be
>>> OUTPUT_STREAMING when firmware start is triggered and substate needs to
>>> be moved to LOAD_RESOURCES.
>>>
>>> Fixes: 547f7b8c5090 ("media: iris: add check to allow sub states transitions")
>>
>> If a restriction has been removed, has that restriction been marked as a 
>> Fixes: ?
>>
>> If not then we can't actually backport this fix as the dependency - the 
>> restriction removal has not been marked for backport.
> 
> Please stop confusing fixes and backports..
> 
> If you're really paranoid about this commit getting autoselected, see
> 'noautosel' under this section:
> 
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Commit still needs to explain what is the bug, what is observable issue
being fixed here. Lack of feature is not a bug.

Best regards,
Krzysztof
Re: [PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming
Posted by Dikshita Agarwal 4 months ago

On 8/14/2025 11:43 AM, Krzysztof Kozlowski wrote:
> On 14/08/2025 00:55, Konrad Dybcio wrote:
>> On 8/13/25 11:51 PM, Bryan O'Donoghue wrote:
>>> On 13/08/2025 10:37, Dikshita Agarwal wrote:
>>>> However, now after removing that restriction, the instance state can be
>>>> OUTPUT_STREAMING when firmware start is triggered and substate needs to
>>>> be moved to LOAD_RESOURCES.
>>>>
>>>> Fixes: 547f7b8c5090 ("media: iris: add check to allow sub states transitions")
>>>
>>> If a restriction has been removed, has that restriction been marked as a 
>>> Fixes: ?
>>>
>>> If not then we can't actually backport this fix as the dependency - the 
>>> restriction removal has not been marked for backport.
>>
>> Please stop confusing fixes and backports..
>>
>> If you're really paranoid about this commit getting autoselected, see
>> 'noautosel' under this section:
>>
>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1
> 
> Commit still needs to explain what is the bug, what is observable issue
> being fixed here. Lack of feature is not a bug.

Ok, will add more info.

Regards,
Dikshita
> 
> Best regards,
> Krzysztof
Re: [PATCH v2 05/24] media: iris: Allow substate transition to load resources during output streaming
Posted by Bryan O'Donoghue 4 months ago
On 13/08/2025 22:51, Bryan O'Donoghue wrote:
>> Fixes: 547f7b8c5090 ("media: iris: add check to allow sub states transitions")
> If a restriction has been removed, has that restriction been marked as a
> Fixes: ?

Yes it has, never mind my proton mail filters are misdirecting patches...

---
bod